add POST account functionality #15
Conversation
| jsonify([Registration.from_db(registration).model_dump(mode="json") for registration in registrations]), | ||
| HTTPStatus.OK, | ||
| ) | ||
| except AuthException as auth_exception: |
There was a problem hiding this comment.
Is this the only kind of exception expected here?
|
|
||
| primary_contact = user | ||
| db.session.add(primary_contact) | ||
| db.session.commit() |
There was a problem hiding this comment.
I think multiple commits should not happen in one request. This should be in a transaction. Either everything is saved or nothing is saved.
There was a problem hiding this comment.
good catch, this should have been .flush()
| """Service to save and load regristration details from the database.""" | ||
|
|
||
| @classmethod | ||
| def save_registration(cls, jwt_oidc_token_info, sbc_account_id, registration_request: requests.Registration): |
There was a problem hiding this comment.
Is this called after payment ?
There was a problem hiding this comment.
payment will be added in a future sprint.
| new_user_account = AuthService.create_user_account(token, name, mailing_address) | ||
| return jsonify(new_user_account), HTTPStatus.CREATED | ||
| registration_request = RegistrationRequest(**json_input) | ||
| selected_account = registration_request.selectedAccount |
There was a problem hiding this comment.
It would be better to move the validation logic to a validator class and return all the validation errors in the request as a list
There was a problem hiding this comment.
moved to RegistrationRequestValidator.py
| ) | ||
| ) | ||
|
|
||
| registration = RegistrationService.save_registration( |
There was a problem hiding this comment.
Why is registration getting saved in the account resource?
There was a problem hiding this comment.
Currently the endpoint is both (potentially) creating SBC user accounts and also saving registrations.
Based on your feedback I have moved it to POST /registrations and I believe in a future sprint we will likely split out the SBC account portion into its own endpoint.
| except AuthException as auth_exception: | ||
| return exception_response(auth_exception) | ||
| except ExternalServiceException as service_exception: | ||
| return exception_response(service_exception) | ||
|
|
||
|
|
||
| def validate_and_format_canadian_postal_code(country: str, province: str, postal_code: str): |
There was a problem hiding this comment.
Please move this to a validator class
There was a problem hiding this comment.
moved to RegistrationRequestValidator.py
| ), | ||
| dateOfBirth=source.rental_property.property_manager.primary_contact_user.date_of_birth, | ||
| details=ContactDetails( | ||
| preferredName=source.rental_property.property_manager.primary_contact_user.preferredname, |
There was a problem hiding this comment.
I see primary and secondary contact as optional. Is primary_contact_user and secondary contact_user always present? If not, primary_contact_user.firstname will throw exception
There was a problem hiding this comment.
Primary Contact is enforced as mandatory in the json schema. Updated the model to reflect that its required.
| db.session.refresh(primary_contact) | ||
|
|
||
| if registration_request.secondaryContact: | ||
| secondary_contact = models.User( |
There was a problem hiding this comment.
Is there a provision to modify this data? Example , If a save functionality is supported, when the application is resumed and when the secondary contact is changed, this will create new user every time?
There was a problem hiding this comment.
There is no edit support currently, that work might come in a later sprint. Currently this method is the only time the data is saved, so there would be no orphaned records. That may change in future work.
Trevor/authapiupdate
Issue:
bcgov/entity#21267
bcgov/entity#21257
bcgov/entity#21258
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namex license (Apache 2.0).